Add a source-only MTP server-mode client package by nohwnd · Pull Request #10085 · microsoft/testfx

Jakub Jareš (nohwnd) · 2026-07-20T13:07:48Z

MTP ships only the server side of its server-mode JSON-RPC protocol today, so consumers that drive an MTP test app have had to maintain bespoke clients. This adds one canonical client, owned in testfx next to the protocol it implements, and ships it as source so vstest, VSUnitTesting, and C# Dev Kit can replace their copies without adding a runtime dependency.

What's here

A new src/Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources project that links the server's protocol and serialization source and adds the client API, JSON-RPC connection, and process launcher.
A source-only Microsoft.Testing.Platform.ServerMode.Client.Sources package: no DLL, no runtime dependency, and all injected types are internal.
Package-private namespaces for linked protocol types, so consumers can reference Microsoft.Testing.Platform.dll without source/assembly type collisions.
Dependency-free, Native AOT-compatible serialization: Jsonite for .NET Framework, netstandard2.0, and net5.0-net7.0 consumers; in-box System.Text.Json for net8.0 and newer.
Synchronous and asynchronous launch APIs, cancellation-aware connection startup, event-safe lazy read-loop startup, and synchronized server-request handlers.
A curated set of down-level polyfills with explicit opt-out constants for consumers that already define common source polyfills.

Validation

Unit coverage exercises initialize, discover, run, filters, notifications, server requests, cancellation, malformed frames, disconnects, and both formatter paths on net462 and modern .NET.
A packed hostile-consumer compile gate covers net462, netstandard2.0, net5.0, net6.0, net7.0, and net8.0 with nullable analysis and warnings-as-errors while also referencing Microsoft.Testing.Platform.
A packed end-to-end consumer launches a real MTP app and verifies discovery and execution over the wire.
Package contract tests verify source-only layout, content-file manifests, namespace isolation, per-TFM formatter selection, curated polyfills, and build assets.
System.Text.Json and Jsonite preserve equivalent untyped numeric representations, including integers through decimal.MaxValue.

Scope

This PR is the testfx/package leg. Adoption in vstest, VSUnitTesting, and C# Dev Kit remains separate so each consumer can remove its bespoke implementation and adapt its repository-specific integration independently.

MTP ships only the server side of its server-mode JSON-RPC protocol today, so every consumer that drives an MTP app has to write its own client. There are three of them: vstest's minimal Jsonite one, VSUnitTesting's mature StreamJsonRpc one, and C# Dev Kit's copy of that. The plan is to own a single client here in testfx and ship it as a source-only package so all three consume the same code. This is the first step - the client and its tests, building and green in-repo. Source-only contentFiles packaging comes later. The client reuses the server's own serialization instead of taking a dependency, so the wire format cannot drift: Jsonite on net462/netstandard, in-box System.Text.Json on .NET. Both are dependency-free and AOT-safe. The net8 leg needed two fixes in the shared STJ decoder, because the server only ever decoded client-to-server requests and never exercised the receive path a client needs: - Register an object[] deserializer. The IDictionary deserializer already binds object[] for array values, but nothing registered it, so any server-to-client message carrying an array (attachments, node changes) killed the read loop. - Keep raw params as an IDictionary for methods the server does not know. The RpcMessage params switch only knew the five server request methods, so client-received notifications dropped their params. Both are behavior-preserving for the server - its serialization tests stay 56/56. Tests run on both formatter paths, net8 (STJ) and net462 (Jsonite), 21/21 each. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Drives a real generated MTP app through the source-only client's MtpServerClient.Launch: initialize, discover, then run in two separate launches, asserting the single action node comes back as discovered and then passed. Runs the net462/net8.0/net10.0 child assets from the net11 host, so the net462 (Jsonite) server talking to the net8 (System.Text.Json) client exercises both formatter paths over the real transport. Also makes the client process launch cross-platform (apphost resolution on Windows/Linux/macOS) and exposes the internals to the acceptance project via an aliased project reference.

Convert Microsoft.Testing.Platform.ServerClient into the source-only package Microsoft.Testing.Platform.ServerClient.Source. It ships the client plus the linked server protocol and serialization source as contentFiles/cs/<tfm>/** (BuildAction=Compile), so consumers compile it as internal types into their own assembly with no shipped DLL and no runtime dependency. The pack target projects the final @(Compile) set into contentFiles, so packed == compiled by construction, and the per-TFM System.Text.Json removal keeps netstandard2.0 Jsonite-only (net462 / netstandard consumers never see the STJ path). Add MtpServerClientSourcePackageTests, the anti-drift contract test: it inspects the produced nupkg and asserts no compiled output, packed == compiled both ways, netstandard2.0 Jsonite-only with net as a superset, the client API present in every target framework, and no polyfill or generated-source leak. Name the readme PACKAGE.md so the shared Directory.Build.targets picks it up. 🤖

Copilot

Pull request overview

Adds a source-only MTP server-mode client package that reuses the platform’s protocol and serialization code.

Changes:

Adds client transport, process-launching, API, and packaging infrastructure.
Extends shared JSON-RPC deserialization for client notifications.
Adds unit, package-contract, and end-to-end acceptance tests.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`TestFx.slnx`	Registers the new projects.
`test/UnitTests/.../TestSetup.cs`	Registers client serializers for tests.
`test/UnitTests/.../Program.cs`	Configures the test executable.
`test/UnitTests/.../MtpServerClientTests.cs`	Tests client protocol behavior.
`test/UnitTests/.../Microsoft.Testing.Platform.ServerClient.UnitTests.csproj`	Configures multi-TFM unit tests.
`test/UnitTests/.../FakeMtpServer.cs`	Implements the loopback fake server.
`test/UnitTests/.../BannedSymbols.txt`	Enforces MSTest assertions.
`test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MtpServerClientAcceptanceTests.cs`	Exercises real MTP applications.
`test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csproj`	References the client project.
`test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientSourcePackageTests.cs`	Validates package contents.
`src/Platform/Microsoft.Testing.Platform/.../Json.Deserializers.cs`	Adds generic arrays and notification parameters.
`src/Platform/Microsoft.Testing.Platform/.../FormatterUtilities.cs`	Selects Jsonite outside .NETCoreApp.
`src/Platform/Microsoft.Testing.Platform.ServerClient/PlatformResourcesShim.cs`	Supplies minimal resource strings.
`src/Platform/Microsoft.Testing.Platform.ServerClient/PACKAGE.md`	Documents package usage.
`src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj`	Defines linked sources and source-only packing.
`src/Platform/Microsoft.Testing.Platform.ServerClient/Client/SerializerUtilities.ClientSerializers.cs`	Adds client serialization directions.
`src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerProcess.cs`	Launches and manages MTP processes.
`src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClientOptions.cs`	Defines client configuration.
`src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClientExceptions.cs`	Defines client exceptions.
`src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClient.cs`	Implements the high-level client.
`src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpJsonRpcConnection.cs`	Implements JSON-RPC correlation and dispatch.
`src/Platform/Microsoft.Testing.Platform.ServerClient/Client/IMtpServerClient.cs`	Defines the client API and models.
`src/Platform/Microsoft.Testing.Platform.ServerClient/Client/IMtpClientLogger.cs`	Defines client diagnostics abstractions.

…rce-package

main added an ILogger (defaulting to NopLogger) to TcpMessageHandler for low-noise transport diagnostics. The source client links that file, so a clean build now needs ILogger, NopLogger, and the LoggingExtensions that define LogDebugAsync. A stale obj hid this locally; the clean CI build failed with CS0246. Link the three logging files. Client unit tests stay green on net8 (STJ) 21/21 and net462 (Jsonite) 21/21, and the source-package contract test passes 5/5. 🤖

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 21 comments.

Comments suppressed due to low confidence (7)

TestFx.slnx:61

The new platform project and its unit-test project are missing from both Microsoft.Testing.Platform.slnf and NonWindowsTests.slnf. Those filters explicitly enumerate the other MTP projects/tests, so product-scoped and non-Windows builds will not compile or test this package. Add both entries to both filters.

    <Project Path="src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj" />

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:182

Excluding generated global usings makes the packed sources depend on undocumented consumer imports. For example, MtpServerProcess.cs uses Process, StringBuilder, and RuntimeInformation without imports because this repo supplies them from Directory.Build.props:143,147,149; SDK implicit usings do not include all of these. An external consumer will fail to compile the content files unless it happens to define the same globals. Ship a package-owned imports source or add explicit imports, and validate the actual nupkg in a consumer with implicit usings disabled.

      - Anything generated into the intermediate obj dir (GlobalUsings.g.cs, *.AssemblyInfo.cs,
        *.AssemblyAttributes.cs, …): each consumer generates its own.

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:126

Compiling these linked files as source does not make their declarations internal. This glob ships many public platform types (TestNode at Messages/TestNode.cs:9, state properties at TestNodeStateProperties.cs:9,56, and others) into every consumer assembly, contradicting the package contract and potentially triggering API-baseline failures or type-conflict warnings in consumers that reference MTP. Use an internalized client model/conditional accessibility rather than packing the public server model verbatim.

  <!-- Data model (TestNode + property model). Glob Messages\ then remove the server-only message bus. -->
  <ItemGroup>
    <Compile Include="$(MTPDir)Messages\*.cs" Link="Linked\Messages\%(Filename)%(Extension)" />

src/Platform/Microsoft.Testing.Platform.ServerClient/PACKAGE.md:28

The shipped source requires newer syntax than C# 9: it uses file-scoped namespaces (C# 10), primary constructors such as PendingRequest(string method), and collection expressions such as ?? [] (C# 12). Either rewrite the package sources to the promised language level or state the actual C# 12 requirement.

- C# language version 9 or later.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/SerializerUtilities.ClientSerializers.cs:35

This idempotence check is not thread-safe, and the flag is set before the dictionaries are fully populated. Two concurrent Launch calls can let one thread observe true and create a System.Text.Json formatter from a partially registered serializer set; the dictionaries are also being read while mutated. Serialize the whole registration operation with a lock/one-time initialization and publish completion only after all entries are installed.

        if (s_clientSerializersRegistered)
        {
            return;
        }

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs:130

Preserving notification params routes test-node payloads through the raw IDictionary decoder, whose number branch uses GetInt32(). The server serializes time.duration-ms as a double (Json.TestNodeSerializer.cs:170), so a normal fractional duration throws while decoding and fails the client's read loop. Decode generic JSON numbers as int/long/double (matching Jsonite) and add a fractional-duration notification test.

                            _ => value.ValueKind == JsonValueKind.Object
                                ? json.Bind<IDictionary<string, object?>>(value)
                                : null,

test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csproj:51

This acceptance test references the validation assembly, not Microsoft.Testing.Platform.ServerClient.Source, so it never exercises NuGet contentFiles selection or compilation into a consumer. The package-inspection test only checks zip structure; neither test would catch missing consumer imports or source-level type conflicts. Consume the packed package from a generated test project and run that output end to end.

    <!-- The MTP server-mode client under test (P4). Referenced through the `serverclient` extern alias so
         its internal TestNode/TestNodeUpdateMessage (same namespaces as the platform's public types) stay
         out of the global namespace and never clash in the non-aliased files of this assembly. -->
    <ProjectReference Include="$(RepoRoot)src\Platform\Microsoft.Testing.Platform.ServerClient\Microsoft.Testing.Platform.ServerClient.csproj" Aliases="serverclient" />

The ServerClient unit test app only registered AddMSTest, so it did not know the --crashdump / --hangdump / --report-trx / --report-ctrf / --report-junit / --report-azdo / --coverage options that test/Directory.Build.targets appends when CI runs every unit test module through 'dotnet test --test-modules'. The module rejected the unknown --hangdump option and exited 5, which the orchestrator reports as 'zero tests ran' and fails the whole leg. Direct console runs never passed --hangdump, so it only reproduced in the full CI run. Register the same provider set every other testfx unit test app registers (CrashDump, HangDump, Trx, JUnit, AzureDevOps, Ctrf, CodeCoverage, OpenTelemetry) so the module accepts those options and runs its 21 tests. Verified by running the built exe directly with the CI options on net8.0 and net462: both exit 0.

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (8)

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:33

This constant is only applied while this project builds; a contentFiles package does not propagate DefineConstants to consumers. The packed ObjectPool.cs therefore takes its #else namespace (Analyzer.Utilities.PooledObjects), while the packed .NET JSON engine references Microsoft.Testing.Platform.Helpers.ObjectPool, so a net8 consumer cannot compile the package. Propagate the constant through packaged build assets or remove the conditional dependency, and validate by compiling a package consumer.

    <DefineConstants>$(DefineConstants);IS_CORE_MTP</DefineConstants>

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:182

The packed sources rely on testfx's generated global usings, but those are deliberately omitted. For example, MtpJsonRpcConnection.cs uses ConcurrentDictionary without importing System.Collections.Concurrent, and MtpServerProcess.cs relies on Process, StringBuilder, and runtime interop imports. Consumer-generated implicit usings do not include all of these, so otherwise valid consumers fail to compile. Add explicit/package-owned usings and compile an actual project from the nupkg.

      - Anything generated into the intermediate obj dir (GlobalUsings.g.cs, *.AssemblyInfo.cs,
        *.AssemblyAttributes.cs, …): each consumer generates its own.

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:126

This glob ships the platform model with its original public accessibility: for example, Messages/TestNode.cs:9 declares public class TestNode, and the linked logging files expose public ILogger/LogLevel. That contradicts the PR/package contract that injected types are internal and can leak duplicate MTP public APIs (and conflict warnings) into consumer assemblies. Internalize/curate the linked contract or explicitly revise the package design and documentation.

  <!-- Data model (TestNode + property model). Glob Messages\ then remove the server-only message bus. -->
  <ItemGroup>
    <Compile Include="$(MTPDir)Messages\*.cs" Link="Linked\Messages\%(Filename)%(Extension)" />

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs:130

The new raw-property-bag path cannot decode all valid server numbers: the generic dictionary/array deserializers call JsonElement.GetInt32(), but real test nodes serialize TimingProperty.GlobalTiming.Duration.TotalMilliseconds as a double. A fractional duration throws while decoding testing/testUpdates/tests, causing the client read loop and pending run to fail. Preserve int/long/double values as appropriate and cover a non-integral duration.

                            _ => value.ValueKind == JsonValueKind.Object
                                ? json.Bind<IDictionary<string, object?>>(value)
                                : null,

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/SerializerUtilities.ClientSerializers.cs:37

Registration is not thread-safe, and the flag is published before the serializer dictionaries are populated. Two concurrent first calls (for example parallel Launch calls in a consumer) can either mutate Dictionary concurrently or let one formatter snapshot a partially registered set. Serialize the entire registration and set the completed flag only after all entries are installed.

        if (s_clientSerializersRegistered)
        {
            return;
        }

        s_clientSerializersRegistered = true;

TestFx.slnx:61

The new platform product and unit-test projects are only added to TestFx.slnx; both are absent from Microsoft.Testing.Platform.slnf and NonWindowsTests.slnf. Product-scoped and non-Windows builds will therefore skip building/packing the client and running its tests. Add both project paths to both filters, following the existing platform project convention.

    <Project Path="src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj" />

src/Platform/Microsoft.Testing.Platform.ServerClient/PACKAGE.md:31

The shipped sources require C# 12 (they use primary constructors and collection expressions), not C# 9. The linked required members also need RequiredMemberAttribute and CompilerFeatureRequiredAttribute polyfills on older targets. Update the consumer requirements so following this documentation produces a compilable project.

- C# language version 9 or later.
- On `net462` / `netstandard2.0`: the usual polyfills (nullable attributes, `IsExternalInit`,
  index/range, `System.HashCode`, `ValueTask`) and framework references (`System.Memory`,
  `System.Threading.Tasks.Extensions`). This package intentionally does **not** ship polyfills, to

test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csproj:51

This acceptance path consumes the validation DLL via ProjectReference, not the source-only nupkg, so it inherits testfx's constants/global usings and never verifies that contentFiles compile in a consumer. The archive-inspection tests cannot catch consumer compilation failures. Generate a small client asset with a PackageReference to the packed Shipping package and drive the server through that compiled asset.

    <!-- The MTP server-mode client under test (P4). Referenced through the `serverclient` extern alias so
         its internal TestNode/TestNodeUpdateMessage (same namespaces as the platform's public types) stay
         out of the global namespace and never clash in the non-aliased files of this assembly. -->
    <ProjectReference Include="$(RepoRoot)src\Platform\Microsoft.Testing.Platform.ServerClient\Microsoft.Testing.Platform.ServerClient.csproj" Aliases="serverclient" />

On non-Windows, eng/Build.props builds and packs NonWindowsTests.slnf, not the full TestFx.slnx. The source-only package project was missing from that filter, so on Linux/macOS it only built transitively (as a dependency of the acceptance tests) and never packed. The acceptance tests then failed with 'Could not find Microsoft.Testing.Platform.ServerClient.Source.*.nupkg'. Add the package project and its unit tests to the filter. The unit tests already restrict net462 to Windows, so on non-Windows they build and run the net8.0 (System.Text.Json) path only. 🤖

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (22)

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:193

The packed source is not self-contained. Files such as MtpJsonRpcConnection.cs and MtpServerProcess.cs use ConcurrentDictionary, Process, StringBuilder, RuntimeInformation, and other types without file-level imports; they compile here only because Directory.Build.props generates repository-wide global usings. This target deliberately excludes generated sources, so a normal external consumer will receive none of those imports and fail compilation. Please add explicit/shipped imports and validate the nupkg in a clean consumer project.

      <_MtpClientPackSource Include="@(Compile)"
                            Condition="'%(Compile.MtpClientDoNotPack)' != 'true' and
                                       !$([System.String]::new('%(Compile.FullPath)').StartsWith('$(_MtpClientIntermediateFullPath)', System.StringComparison.OrdinalIgnoreCase))" />

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:126

This glob ships the platform message declarations with their original accessibility. For example, Messages/TestNode.cs:9 and TestNodeUpdateMessage.cs:14 are public, so NuGet does not compile the injected source “as internal”; it adds duplicate public MTP types to every consumer and can shadow types from Microsoft.Testing.Platform. Please make the source-package copies internal (or avoid shipping duplicate model declarations) before publishing.

  <!-- Data model (TestNode + property model). Glob Messages\ then remove the server-only message bus. -->
  <ItemGroup>
    <Compile Include="$(MTPDir)Messages\*.cs" Link="Linked\Messages\%(Filename)%(Extension)" />

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/SerializerUtilities.ClientSerializers.cs:37

Registration is not thread-safe, and the flag is published before the dictionaries are fully populated. Two concurrent Launch calls can let one thread create a formatter from a partial serializer snapshot while the other mutates the shared Dictionary instances. Serialize initialization under a lock and set the completed flag only after every registration has finished.

        if (s_clientSerializersRegistered)
        {
            return;
        }

        s_clientSerializersRegistered = true;

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs:130

Preserving unknown notification params now routes telemetry and test-node property bags through the generic decoder, but that decoder uses GetInt32() for every JSON number (including the new array path). The server serializer explicitly emits long, float, double, and decimal; a duration or non-integral telemetry metric therefore throws and terminates the client's read loop. Decode the supported numeric shapes without narrowing, and cover a double/long notification.

                            _ => value.ValueKind == JsonValueKind.Object
                                ? json.Bind<IDictionary<string, object?>>(value)
                                : null,

src/Platform/Microsoft.Testing.Platform.ServerClient/PACKAGE.md:28

The shipped client already uses C# 12 syntax, including primary constructors (DelegateMtpClientLogger and PendingRequest) and collection expressions. A consumer compiling with C# 9 cannot parse the package sources, so this requirement is incorrect.

- C# language version 9 or later.

TestFx.slnx:61

The new platform product and its unit tests are added to the full and non-Windows solutions, but both are absent from Microsoft.Testing.Platform.slnf (currently lines 8-35). Product-scoped platform builds therefore skip this package and its tests. Add both project paths to that filter as well.

    <Project Path="src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj" />

test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csproj:51

This ProjectReference makes the end-to-end test run against the built DLL under testfx's global usings, polyfills, and IS_CORE_MTP; it never restores or compiles Microsoft.Testing.Platform.ServerClient.Source. Consequently the test named ViaSourcePackageClient cannot catch source-package consumer failures. Build a clean generated asset with a PackageReference to the packed nupkg and drive that client instead.

    <!-- The MTP server-mode client under test (P4). Referenced through the `serverclient` extern alias so
         its internal TestNode/TestNodeUpdateMessage (same namespaces as the platform's public types) stay
         out of the global namespace and never clash in the non-aliased files of this assembly. -->
    <ProjectReference Include="$(RepoRoot)src\Platform\Microsoft.Testing.Platform.ServerClient\Microsoft.Testing.Platform.ServerClient.csproj" Aliases="serverclient" />

test/UnitTests/Microsoft.Testing.Platform.ServerClient.UnitTests/TestSetup.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

test/UnitTests/Microsoft.Testing.Platform.ServerClient.UnitTests/Program.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

test/UnitTests/Microsoft.Testing.Platform.ServerClient.UnitTests/MtpServerClientTests.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

test/UnitTests/Microsoft.Testing.Platform.ServerClient.UnitTests/FakeMtpServer.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MtpServerClientAcceptanceTests.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientSourcePackageTests.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/PlatformResourcesShim.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/SerializerUtilities.ClientSerializers.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerProcess.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClientOptions.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClientExceptions.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClient.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpJsonRpcConnection.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/IMtpServerClient.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/IMtpClientLogger.cs:1

This new C# file is saved without the required UTF-8 BOM. .editorconfig:65-68 sets charset = utf-8-bom for all C# files; please resave it with the configured encoding.

// Copyright (c) Microsoft Corporation. All rights reserved.

The source-only ServerClient package embeds the server's Jsonite under a top-level `namespace Jsonite`. vstest already has its own internal top-level `namespace Jsonite`, so on net462/netstandard2.0 both copies compile into CrossPlatEngine and collide (CS0436), failing vstest's warnings-as-errors build. Move it under `Microsoft.Testing.Platform.ServerMode.JsonRpc.Json.Jsonite` (matches the folder). Pure namespace move, no wire-format or behavior change: the formatter Id stays "Jsonite" and the JSON output is identical. Server and client compile from the same files, so the rename is unconditional. Validated: platform + client unit tests (net462 Jsonite + net8 STJ 21/21 each, platform 1371/1393), the packed==compiled contract test (5/5), and the real-app acceptance test (3/3) all green. 🤖

Copilot

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (20)

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:33

DefineConstants only affects this validation project; it is not propagated with contentFiles. A package consumer therefore compiles ObjectPool.cs without IS_CORE_MTP, placing ObjectPool<T> in Analyzer.Utilities.PooledObjects (Helpers/ObjectPool.cs:21-25), while the packed Json/Json.cs imports Microsoft.Testing.Platform.Helpers and instantiates that type. The net8 source package will not compile. Propagate the symbol through package build assets or remove the conditional namespace dependency from the shipped source.

    <DefineConstants>$(DefineConstants);IS_CORE_MTP</DefineConstants>

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:182

Skipping generated global usings makes the packed source depend on testfx's Directory.Build.props, which consumers do not receive. For example, MtpJsonRpcConnection.cs uses ConcurrentDictionary without importing System.Collections.Concurrent, MtpServerProcess.cs uses Process/StringBuilder without their namespaces, and the non-.NET path relies on the project-only Polyfills using. The nupkg therefore fails to compile in a normal consumer. Add explicit imports to shipped files (or a compatible packaged imports mechanism).

    Skipped:
      - Polyfills (MtpClientDoNotPack=true): consumers already provide their own.
      - Anything generated into the intermediate obj dir (GlobalUsings.g.cs, *.AssemblyInfo.cs,
        *.AssemblyAttributes.cs, …): each consumer generates its own.

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs:74

This generic decoder rejects valid server numbers that are not Int32. In particular, test-node serialization emits TimingProperty.GlobalTiming.Duration.TotalMilliseconds as a double (Json.TestNodeSerializer.cs:168-170), so an ordinary timed test update makes GetInt32() throw and terminates the client read loop. The dictionary-number branch above has the same limitation. Decode int, long, and floating-point JSON numbers in both branches.

                    case JsonValueKind.Number:
                        items.Add(element.GetInt32());

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/SerializerUtilities.ClientSerializers.cs:37

The idempotence guard is not thread-safe. If two clients launch concurrently, one thread can observe true while the first is still mutating the shared serializer dictionaries, then snapshot an incomplete set in CreateFormatter; requests later fail due to missing serializers. Synchronize the entire registration and publish the completed state only after all entries are installed.

        if (s_clientSerializersRegistered)
        {
            return;
        }

        s_clientSerializersRegistered = true;

src/Platform/Microsoft.Testing.Platform.ServerClient/PACKAGE.md:28

The shipped source uses C# 12 features, including collection expressions ([]) and primary constructors, so it cannot compile with the documented C# 9 minimum. Either rewrite the injected source to C# 9 syntax or state the actual minimum.

- C# language version 9 or later.

test/UnitTests/Microsoft.Testing.Platform.ServerClient.UnitTests/TestSetup.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.
test/UnitTests/Microsoft.Testing.Platform.ServerClient.UnitTests/Program.cs:1
This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.
test/UnitTests/Microsoft.Testing.Platform.ServerClient.UnitTests/MtpServerClientTests.cs:1
This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.
test/UnitTests/Microsoft.Testing.Platform.ServerClient.UnitTests/FakeMtpServer.cs:1
This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MtpServerClientAcceptanceTests.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientSourcePackageTests.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/PlatformResourcesShim.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/SerializerUtilities.ClientSerializers.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerProcess.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClientOptions.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClientExceptions.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClient.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpJsonRpcConnection.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/IMtpServerClient.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/IMtpClientLogger.cs:1

This new C# file is UTF-8 without a BOM, but .editorconfig:66-67 requires utf-8-bom for all C# files. Resave it with a BOM.

// Copyright (c) Microsoft Corporation. All rights reserved.

…e MTP client MtpTestNodeUpdate now decodes standardOutput, standardError, and the location.file/line-start/line-end wire keys into StandardOutput, StandardError, FilePath, LineStart, and LineEnd, so consumers stop reaching into the raw Node bag for the common fields. Line numbers arrive as JSON numbers, so a small coercion handles whichever numeric type each formatter boxes them as. Also documents the discover/run ordering guarantee: once the returned task completes every TestNodesUpdated handler has already run, so consumers do not need a settle delay or completion sentinel. This replaces the old fixed wait the vstest client used. Tested on both formatter paths (net8 System.Text.Json, net462 Jsonite): unit 22/22 each, contract 5/5, acceptance 3/3. 🤖

Copilot

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/Platform/Microsoft.Testing.Platform.ServerClient/PACKAGE.md:29

This understates the compiler requirement. The package ships Polyfills/OperatingSystem.cs, whose active net462/netstandard2.0 branch uses a C# 14 extension block (extension(OperatingSystem) at line 15). With a C# 12 or 13 compiler, the packaged target sets LangVersion=latest but the injected source still fails to parse. Either avoid that C# 14 syntax in shipped source or document C# 14 as the minimum.

- C# language version 12 or later (the shipped source uses collection expressions and other C# 12
  features).

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpJsonRpcConnection.cs:323

The self-wait guard is unreliable for this async loop. Task.Run(Func<Task>) stores an unwrapped proxy task, while Task.CurrentId inside an async continuation is not guaranteed to equal that proxy's ID (and is commonly null). If an event or server-request handler calls Dispose, this can therefore wait five seconds on the read loop that is currently executing the handler. Track an explicit read-loop/dispatch context or avoid synchronously waiting when disposal originates from a callback.

        Task? readLoop = _readLoop;
        if (readLoop is not null && Task.CurrentId != readLoop.Id)
        {
            try
            {
                readLoop.Wait(ReadLoopShutdownTimeout);

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs:326

This fallback does not actually match Jsonite for all valid JSON integers. After ulong, Jsonite tries decimal (Jsonite/JsonReader.cs:519-523), whereas this path converts directly to double; an integer such as decimal.MaxValue is therefore preserved on the Jsonite TFM but rounded on the System.Text.Json TFM. Untyped telemetry/property-bag values can consequently differ or lose precision. Preserve decimal for integer-form tokens beyond ulong, while retaining double for fractional/exponent tokens.

        if (element.TryGetUInt64(out ulong ulongValue))
        {
            return ulongValue;
        }

        return element.GetDouble();

- AsInt: test double integrality with the constant pattern d % 1d is 0d instead of d == Math.Floor(d), so the code-scanning float-equality rule does not fire (behaviorally identical). - MtpJsonRpcConnection.Dispose: guard the read-loop self-wait with an AsyncLocal<bool> flow marker instead of Task.CurrentId. ReadLoopAsync is async, so after its first await Task.CurrentId no longer matches the loop's task id and a handler-triggered Dispose would self-wait for the full 5s shutdown timeout. Adds a regression test. - MtpServerProcess: cap the retained standard-error buffer at 64 KB with a front-trim so a chatty/long-lived server cannot grow it without bound; the tail (most relevant near a crash) is kept. - PACKAGE.md: correct the C# language-version note (build targets default LangVersion=latest; a pinned version needs C# 14 on net462/netstandard2.0). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 519bdc9e-ace7-46fa-aa7e-d07eeef955f6

Copilot

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClientOptions.cs:36

The summary says false makes the client perform one operation and then exit, but the implementation only sends this value during initialization; it never auto-exits after discover/run. The remarks below describe the actual behavior, so the summary should not promise lifecycle behavior the option does not implement.

    /// Gets or sets a value indicating whether the client keeps the connection alive for multiple requests
    /// (<c>capabilities.testing.isStateful</c> / <c>experimental_multiRequestSupport</c>). When
    /// <see langword="false"/> the client performs a single discover or run and then exits. Defaults to
    /// <see langword="false"/>.

src/Platform/Microsoft.Testing.Platform.ServerClient/build/Microsoft.Testing.Platform.ServerClient.Source.targets:26

This condition also matches a consumer that explicitly pins C# 7.3, so the package silently overrides that explicit choice despite the comment saying explicit choices are never overridden. That can change compilation semantics for the consumer's own source. Only supply latest when LangVersion is unset; an explicitly incompatible version should remain intact and fail with a clear compatibility diagnostic.

    <LangVersion Condition="'$(LangVersion)' == '' or '$(LangVersion)' == '7.3'">latest</LangVersion>

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/TcpMessageHandler.cs:34

The PR description states that only FormatterUtilities.cs and Json.Deserializers.cs change on the shared server side, but this hunk rewrites the server transport framing, and the diff also changes IMessageFormatter, Json.cs, Json.TestNodeSerializer.cs, and a shared polyfill. Please update the description and server-side test summary so reviewers and release notes reflect the actual compatibility surface being changed.

    // The read side deliberately does NOT use a StreamReader. Content-Length is declared in UTF-8 *bytes*
    // (see WriteRequestAsync), so the body must be consumed as bytes and decoded afterwards. A StreamReader
    // hands out decoded characters, which for multi-byte UTF-8 content are fewer units than the declared
    // length: the reader under-reads the frame, leaves its tail in the stream, and the framing permanently
    // desynchronizes from the next frame onwards. Reading the headers through a StreamReader and the body
    // from BaseStream would be worse still, because the reader's internal buffer would have already
    // swallowed part of the body. Headers and body are therefore both read through this one byte-level
    // buffer, so nothing can be buffered on the other side of the boundary.

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:355

The transform writes these generated files under obj but never records them in @(FileWrites), so MSBuild's Clean target does not know to remove them. Register the transformed outputs after the task, as other generated targets in this repository do (for example Microsoft.Testing.Platform.MSBuild.targets:56).

    <!-- Write the transformed copies to obj. -->
    <_MtpClientTransformSource Files="@(_MtpClientTransformed)" />

The server-mode IMessageFormatter/MessageFormatter/Json.Deserialize<T> overloads changed from ReadOnlyMemory<char> to ReadOnlyMemory<byte> (the byte/char framing fix). Record that in net/InternalAPI.Unshipped.txt so PublicApiAnalyzers stops reporting the removed char overloads (RS0017) and the new byte overloads (RS0016): *REMOVED* the three char signatures that net/InternalAPI.Shipped.txt still lists, and declare the three byte ones. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 519bdc9e-ace7-46fa-aa7e-d07eeef955f6

Copilot

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs:326

ReadNumber does not fully mirror Jsonite as documented: Jsonite falls back to decimal for integral values outside ulong but within decimal (JsonReader.cs:519-523), while this fallback converts them to double and loses precision. Preserve that integer case before using GetDouble().

        return element.GetDouble();

src/Platform/Microsoft.Testing.Platform.ServerClient/build/Microsoft.Testing.Platform.ServerClient.Source.targets:49

Appending CS0436 to the consumer project's global NoWarn suppresses every source-vs-imported-type conflict in adopter code, not only collisions from this package's polyfills. Scope the suppression to the transformed package source (for example, via a generated #pragma) or exclude only the colliding polyfills so unrelated conflicts remain visible.

    <NoWarn>$(NoWarn);CS0436</NoWarn>

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClientOptions.cs:36

This describes behavior the client does not implement: with the default false, discover/run return without sending exit, and callers/tests explicitly call ExitAsync. State that this value is only advertised during initialization and that request sequencing and shutdown remain the caller's responsibility.

    /// <summary>
    /// Gets or sets a value indicating whether the client keeps the connection alive for multiple requests
    /// (<c>capabilities.testing.isStateful</c> / <c>experimental_multiRequestSupport</c>). When
    /// <see langword="false"/> the client performs a single discover or run and then exits. Defaults to
    /// <see langword="false"/>.

src/Platform/Microsoft.Testing.Platform.ServerClient/build/Microsoft.Testing.Platform.ServerClient.Source.targets:26

This condition cannot distinguish the framework's 7.3 default from a consumer that explicitly pinned C# 7.3, so the package silently overrides an explicit project choice despite the comment and package documentation. Provide the conditional default from a packaged .props file ('$(LangVersion)' == '') so the consumer project can override it, and keep late composition logic in .targets.

    <LangVersion Condition="'$(LangVersion)' == '' or '$(LangVersion)' == '7.3'">latest</LangVersion>

src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpJsonRpcConnection.cs:263

Only send $/cancelRequest when cancellation actually wins the completion race. Currently, if the response completes and the token fires before the pending entry is removed, TrySetCanceled fails but a stale cancel notification is still sent for an already-completed request.

        pending.Completion.TrySetCanceled(cancellationToken);

        // Best-effort notify the server to stop the in-flight work.
        _ = SendCancelNotificationAsync(id);

Resolve the InternalAPI.Unshipped.txt conflict by keeping both sides: the server-mode Deserialize byte-signature updates from this branch and the AsyncConsumerDataProcessor constructor entry from main. The FormatterUtilitiesTests and Json.TestNodeSerializer auto-merges reconcile cleanly: main added tests that route through the private Deserialize<T>(string) helper, which this branch changed to convert to UTF-8 bytes on NETCOREAPP. Verified on the merged tree: full pack build green (0 warnings, 0 errors), Microsoft.Testing.Platform.ServerClient.Source packs, and the ServerMode FormatterUtilities tests pass 40/40 on net8.0. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 519bdc9e-ace7-46fa-aa7e-d07eeef955f6

Copilot

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated no new comments.

Suppressed comments (7)

src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/Json/Json.Deserializers.cs:326

This fallback does not actually match Jsonite for integral values beyond UInt64: Jsonite next returns decimal (JsonReader.cs:515-520), while this converts the token to double and loses precision. Preserve the remaining integer-token case as decimal before using the floating-point fallback.

        return element.GetDouble();

src/Platform/Microsoft.Testing.Platform.ServerClient/build/Microsoft.Testing.Platform.ServerClient.Source.targets:49

NoWarn is a project-wide compiler setting, so merely referencing this package suppresses every CS0436 in the adopter's own code and can hide unrelated source/import type conflicts. Scope the suppression to the generated package files instead—for example, prepend #pragma warning disable CS0436 in the source transform—and leave the consumer's global warning policy unchanged.

    <NoWarn>$(NoWarn);CS0436</NoWarn>

test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientPackagedConsumerRunTests.cs:328

This second generated project path has the same argument-splitting problem when the asset root contains spaces. Quote it before passing the command to dotnet build.

            $"build {testAsset.TargetAssetPath}/PackagedConsumer -c {Constants.BuildConfiguration}",

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:87

Globbing the entire repository polyfill set is not safe for a source-injected package. On modern .NET many of these files take their #else branch and emit assembly-level TypeForwardedTo attributes (for example IsExternalInit.cs:19 and RequiredMemberAttribute.cs:25), so they do not “compile to nothing” and instead add exported type forwarders to every adopter assembly. Down-level, only the OS and Range/Index files have EXCLUDE_* guards, so an adopter that already defines common source polyfills gets duplicate-type errors that NoWarn=CS0436 cannot suppress. Curate package-safe polyfills or add package-specific guards, and cover a consumer with existing source polyfills plus public-API analysis.

    <Compile Include="$(RepoRoot)src/Polyfills/**/*.cs" Link="Polyfills\%(RecursiveDir)%(Filename)%(Extension)" />

src/Platform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj:64

The package-specific text needs to lead the description, with $(CommonProductDescription) appended last. This is the repository's stated pack metadata convention (Directory.Build.targets:65-66) and is followed by peer platform packages such as Microsoft.Testing.Extensions.HtmlReport.csproj:11-13; hard-coding the shared sentence first also lets this package drift when the shared description changes.

    <PackageDescription>
      <![CDATA[Microsoft Testing is a set of platform, framework and protocol intended to make it possible to run any test on any target or device.

This is a source-only package: it injects (as internal source) a client for the Microsoft Testing Platform (MTP) server-mode JSON-RPC protocol, sharing the exact protocol and serialization source the platform server compiles. It has no runtime dependency and is native-AOT friendly (Jsonite on .NET Framework / netstandard2.0, in-box System.Text.Json on .NET).]]>
    </PackageDescription>

test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientSourcePackageConsumerTests.cs:157

The generated asset path is not quoted, so this build command is split incorrectly whenever the repository or temporary asset root contains spaces. Quote the project path as the other acceptance-test build invocations do.

            $"build {testAsset.TargetAssetPath}/HostileConsumer -c {Constants.BuildConfiguration}",

test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientPackagedConsumerRunTests.cs:314

This generated project path is unquoted, so the acceptance test cannot build from a checkout or asset directory containing spaces. Pass the path as one quoted command-line argument.

This issue also appears on line 328 of the same file.

            $"build {testAsset.TargetAssetPath}/DummyApp -c {Constants.BuildConfiguration}",

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7194280a-9169-44e0-a35c-466c64385a12

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7194280a-9169-44e0-a35c-466c64385a12

github-actions · 2026-08-07T00:39:30Z

Parallel-safety audit — PR #10085

Scope note: the workflow's pre-extracted file/line-range lists were unavailable in this run, so I pulled the PR diff directly via the GitHub API. Almost every changed test file in this PR is newly added, so the primary/pre-existing distinction mostly collapses: findings below are primary unless explicitly marked pre-existing/context.

Step 0 — Parallelization state per affected assembly

Assembly	Opt-in source	Effective scope	Workers	Analyzer coverage
`Microsoft.Testing.Platform.ServerMode.Client.Sources.UnitTests` (new, added by this PR)	`[assembly: Parallelize(Scope = MethodLevel, Workers = 0)]` in new `Program.cs`	MethodLevel	0 (CPU count)	Coverable once MSTEST0074‐0077 ship (plain attribute, compiler-visible) — not active today, only MSTEST0073 ships on `main`
`Microsoft.Testing.Platform.UnitTests` (existing, `ServerMode/*Tests.cs` modified)	Pre-existing `[assembly: Parallelize(Scope = MethodLevel, Workers = 0)]` in its own `Program.cs`	MethodLevel	0	Unchanged by this PR
`MSTest.Acceptance.IntegrationTests` (existing, new file added)	Pre-existing `[assembly: Parallelize(Scope = MethodLevel, Workers = 0)]` in `Program.cs`	MethodLevel	0	Unchanged by this PR
`Microsoft.Testing.Platform.Acceptance.IntegrationTests` (existing, 2 new files added)	Pre-existing `[assembly: Parallelize(Scope = MethodLevel, Workers = 0)]` in `Program.cs`	MethodLevel	0	Unchanged by this PR

No .runsettings/testconfig.json/MSBuild override was found for any of these assemblies, and this PR touches no Directory.Build.props/.targets. MethodLevel means both intra-class and cross-class conflicts would be live in every assembly this PR adds tests to — so the isolation quality of the new tests matters.

Findings

No Critical/High findings. The new tests follow strong isolation patterns throughout:

Ephemeral ports, not fixed ports (good pattern, not a finding). Both FakeMtpServer (unit tests) and TcpMessageHandlerTests.ConnectedHandlers (existing project, new helper) bind via new TcpListener(IPAddress.Loopback, 0). Port 0 is OS-assigned, so concurrent instances never collide — this correctly avoids what would otherwise be a category-B shared-fixed-resource hazard under MethodLevel.
Per-test fixture instantiation. Every method in MtpServerClientTests.cs (~30 methods) creates its own using FakeMtpServer server = new(); — no shared mutable fixture across methods, no [ResourceLock]/[DoNotParallelize] needed or missing.
Child-process environment, not process-global. MtpServerClientAcceptanceTests.CreateOptions() and MtpServerClientPackagedConsumerRunTests.CreateChildEnvironment() both build a Dictionary<string, string?> passed into a launched child process's environment (MtpServerClientOptions.EnvironmentVariables, or DotnetCli.RunAsync(..., environmentVariables: ...)). Neither calls Environment.SetEnvironmentVariable on the current test-host process, so this is not a category‐A finding — the current process's environment/CWD is never mutated.
Read-only shared static field — not a hazard. MtpServerClientSourcePackageTests has private static readonly SourcePackage Package = SourcePackage.Load(); shared across its test methods. SourcePackage.Load() only reads a .nupkg from artifacts/packages/<Configuration>/Shipping (via ZipFile.OpenRead) once, and every subsequent access is read-only (Package.AllEntries, Package.PackedCsByTfm, ...). No mutation, so no [DoNotParallelize] is needed for this class despite the repo convention about shared mutable generated assets — this asset is immutable after load.
Isolated NuGet restore per test. MtpServerClientPackagedConsumerRunTests/MtpServerClientSourcePackageConsumerTests use Path.Combine(testAsset.TargetAssetPath, ".nuget-packages") — a path unique to each test's own TestAsset (via AssetName/GenerateAssetAsync), not a shared fixed path across methods — so no category‐B collision.
Context/Info only: the new TestSetup.cs [AssemblyInitialize] calls SerializerUtilities.RegisterClientSerializers(), which mutates a shared static registration dictionary. This is assembly-fixture code, serialized once by MSTest's own semaphore before any worker runs — not a live race — and the production method itself uses double-checked locking (ClientSerializersLock + volatile flag), so it's also safe if ever invoked from elsewhere. No action needed.
Context/Info only: Environment.SetEnvironmentVariable("DOTNET_CLI_TELEMETRY_OPTOUT", "1") in the new Program.cs executes as a top-level statement before the test host starts, mirroring every other MSTest-based unit-test Program.cs in this repo — one-time process bootstrap, not a per-test mutation, so not a live category-A race.

Category D (over-serialization)

No over-serialization concerns: no new [DoNotParallelize] was added on a method/class that didn't need it, and no unnecessarily broad [ResourceLock] was introduced. All Workers values found are either 0 (CPU count) or explicit positive counts pre-existing in ParallelExecutionTests.cs/ResourceLockExecutionTests.cs, none touched by this PR.

Bottom line

This PR introduces a new MethodLevel-parallel test assembly plus new tests in three existing MethodLevel-parallel assemblies. I found no process-global-state races, no shared-path collisions, and no [ResourceLock]/[DoNotParallelize] declaration mismatches — the new tests consistently isolate their shared resources (ephemeral ports, per-test fixtures, child-process env vars, immutable cached artifacts). No changes are recommended from a parallel-safety standpoint.

(Cross-ref: testability/smell/anti-pattern concerns, if any, are covered by the sibling detect-static-dependencies/test-smell-detection/test-anti-patterns analyses and are out of scope here.)

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 179.4 AIC · ⌖ 3.5 AIC · ⊞ 24.6K · [◷]( · ◷)

Amaury Levé (Evangelink)

Reviewed the package architecture, source transforms, compatibility matrix, concurrency, cancellation, and end-to-end behavior after the merge-readiness fixes. The remaining findings were addressed and the targeted unit, package-consumer, and cross-platform validation is green.

github-actions · 2026-08-07T01:06:46Z

🧪 Expert test review — PR #10085

Grade	Test	Mutation	Notes	How to improve
B (80–89)	new `MtpServerClientSourcePackageConsumerTests. HostileConsumer_ CompilesAgainstPackedSource`	N/A	Single ExitCode==0 assertion is appropriate for a compile oracle, but stderr diagnostics aren't asserted beyond the failure message.	Also assert result.StandardError is empty/does not contain "error" to catch warnings-as-errors silently swallowed by a non-zero-but-untested path.
A (90–100)	new `MtpServerClientAcceptanceTests. DiscoverAndRun_ ViaSourcePackageClient_ ReportsExpectedTestNode`	N/A	Two independent client sessions (discover, then run) with precise ContainsSingle assertions and descriptive failure messages.	—
A (90–100)	new `MtpServerClientPackagedConsumerRunTests. PackagedConsumer_ LaunchesRealServer_ DiscoversAndRunsExpectedNode`	N/A	End-to-end build + run gate asserts exit code and each discrete stdout marker (DISCOVERED/EXECUTED/OK), giving good failure isolation.	—

Summary: Three new acceptance tests were added covering the new Microsoft.Testing.Platform.ServerMode.Client.Sources package: an in-repo client acceptance test, a packaged-consumer end-to-end run test, and a hostile-consumer compile oracle. All three follow existing acceptance-test conventions (asset generation, Assert.AreEqual/Assert.Contains/Assert.ContainsSingle with descriptive messages, isolated NuGet caches to avoid stale-package false passes). No swallowed exceptions, no tautological assertions, and no reliability/isolation issues were found (each test uses its own generated asset directory). No inline suggestions were posted — the sole noted improvement is a minor enhancement rather than a defect.

_{This advisory comment was generated automatically. Grades are heuristic

and informational — they do not block merging. Suggestions on the Files

changed tab can be applied with one click. Re-run with

/review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 61.5 AIC · ⌖ 3.4 AIC · ⊞ 16.7K · [◷]( · ◷)

Jakub Jareš (nohwnd) and others added 3 commits July 15, 2026 15:23

Copilot AI balanced review requested due to automatic review settings July 20, 2026 13:07

Copilot started reviewing on behalf of Jakub Jareš (nohwnd) July 20, 2026 13:08 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

github-code-quality Bot found potential problems Jul 20, 2026

View reviewed changes

Jakub Jareš (nohwnd) added 2 commits July 20, 2026 15:20

Merge remote-tracking branch 'origin/main' into nohwnd-mtp-client-sou…

d33c04f

…rce-package

Copilot AI review requested due to automatic review settings July 20, 2026 13:25

Copilot started reviewing on behalf of Jakub Jareš (nohwnd) July 20, 2026 13:26 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Copilot AI review requested due to automatic review settings July 20, 2026 14:32

Copilot started reviewing on behalf of Jakub Jareš (nohwnd) July 20, 2026 14:33 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 20, 2026 14:46

Copilot started reviewing on behalf of Jakub Jareš (nohwnd) July 20, 2026 14:47 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Comment thread ...Platform/Microsoft.Testing.Platform.ServerMode.Client.Sources/Client/MtpJsonRpcConnection.cs

Amaury Levé (Evangelink) reviewed Jul 21, 2026

View reviewed changes

Comment thread ...tform/Microsoft.Testing.Platform.ServerClient/Microsoft.Testing.Platform.ServerClient.csproj Outdated

Amaury Levé (Evangelink) reviewed Jul 21, 2026

View reviewed changes

Comment thread src/Platform/Microsoft.Testing.Platform.ServerClient/Client/MtpServerClient.cs Outdated

Copilot AI review requested due to automatic review settings July 21, 2026 08:55

Copilot started reviewing on behalf of Jakub Jareš (nohwnd) July 21, 2026 08:55 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread .../Microsoft.Testing.Platform.Acceptance.IntegrationTests/MtpServerClientSourcePackageTests.cs

Jakub Jareš (nohwnd) mentioned this pull request Jul 21, 2026

Retarget the MTP client onto Microsoft.Testing.Platform.ServerMode.Client.Sources microsoft/vstest#16300

Open

Copilot AI review requested due to automatic review settings July 21, 2026 09:10

Copilot started reviewing on behalf of Jakub Jareš (nohwnd) August 4, 2026 12:22 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 4, 2026 12:45

Copilot started reviewing on behalf of Jakub Jareš (nohwnd) August 4, 2026 12:45 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Comment thread src/Platform/Microsoft.Testing.Platform/ServerMode/JsonRpc/IMessageFormatter.cs

Copilot AI review requested due to automatic review settings August 4, 2026 13:09

Copilot started reviewing on behalf of Jakub Jareš (nohwnd) August 4, 2026 13:10 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 6, 2026 08:18

Copilot started reviewing on behalf of Jakub Jareš (nohwnd) August 6, 2026 08:19 View session

Copilot AI reviewed Aug 6, 2026

View reviewed changes

Finish MTP server-mode client package

6dfe458

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7194280a-9169-44e0-a35c-466c64385a12

Copilot AI review requested due to automatic review settings August 6, 2026 16:22

Amaury Levé (Evangelink) marked this pull request as ready for review August 6, 2026 16:24

Copilot AI reviewed Aug 6, 2026

github-code-quality Bot found potential problems Aug 6, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Amaury Levé (Evangelink) self-requested a review August 6, 2026 23:53

Retry CI after GitHub outage

90b1d00

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7194280a-9169-44e0-a35c-466c64385a12

Amaury Levé (Evangelink) enabled auto-merge (squash) August 7, 2026 00:39

Amaury Levé (Evangelink) approved these changes Aug 7, 2026

View reviewed changes

Amaury Levé (Evangelink) merged commit 3a64386 into main Aug 7, 2026
42 of 43 checks passed

Amaury Levé (Evangelink) deleted the nohwnd-mtp-client-source-package branch August 7, 2026 01:11

This was referenced Aug 7, 2026

[docs] Update glossary - daily scan #10496

Closed

[docs] Document MTP server mode #10499

Merged

Uh oh!

Conversation

Jakub Jareš (nohwnd) commented Jul 20, 2026 • edited by Evangelink Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's here

Validation

Scope

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jakub Jareš (nohwnd) commented Jul 20, 2026 •

edited by Evangelink

Loading